chore: started replacing exceptions#3867
chore: started replacing exceptions#3867noellie-velez wants to merge 6 commits intodevelop-2.0.0from
Conversation
| // prior to invoking OnNetworkSpawn so cross NetworkBehaviour: | ||
| // - accessing of NetworkVariables will work correctly. | ||
| // - invocation of RPCs will work properly (and not throw exception under certain scenarios) | ||
| // - invocation of RPCs will work properly (and not throw exception under certain scenarios) CHECK this comment about exceptions |
There was a problem hiding this comment.
as we are removing exceptions we should remove (and not throw exception under certain scenarios), right?
| if (NetworkManager.Singleton.LogLevel <= LogLevel.Error) | ||
| { | ||
| NetworkLog.LogError("Could not serialize NetworkObject: Out of buffer space."); | ||
| } |
There was a problem hiding this comment.
I didn't catch why we didn't want to add a return here, is it correct not to add one for throwing an overflow exception?
There was a problem hiding this comment.
I think I got mixed up. I was saying maybe we leave the exceptions in. I'll check with Noel what we want the behaviour to be here.
| if (NetworkManager.Singleton.LogLevel <= LogLevel.Error) | ||
| { | ||
| NetworkLog.LogError("Could not serialize NetworkObject: Out of buffer space."); | ||
| } |
There was a problem hiding this comment.
I think I got mixed up. I was saying maybe we leave the exceptions in. I'll check with Noel what we want the behaviour to be here.
| if (!writer.TryBeginWrite(writeSize)) | ||
| { | ||
| throw new OverflowException("Could not serialize SceneObject: Out of buffer space."); | ||
| if (NetworkManager.Singleton.LogLevel <= LogLevel.Error) |
There was a problem hiding this comment.
We prefer to not use the Singleton in our internal code because the integration tests run with multiple NetworkManagers running at the same time. That means we need to be sure that the right NetworkManager is being used in each place. In this spot you can access the correct NetworkManager using OwnerObject.NetworkManagerOwner.
2 participants
Purpose of this PR
Jira ticket
Link to related jira ticket (Use the smart commits). Short version (e.g. MTT-123) also works and gets auto-linked
Changelog
Documentation
Testing & QA (How your changes can be verified during release Playtest)
Functional Testing
Manual testing :
Manual testing doneAutomated tests:
Covered by existing automated testsCovered by new automated testsDoes the change require QA team to:
Review automated tests?Execute manual tests?Provide feedback about the PR?If any boxes above are checked the QA team will be automatically added as a PR reviewer.
Backports